Premium Analytics: converge the duplicated date-label validators and day bounds - #52070
Premium Analytics: converge the duplicated date-label validators and day bounds#52070chihsuan wants to merge 3 commits into
Conversation
chihsuan
changed the title
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! |
Code Coverage SummaryCoverage changed in 6 files. Only the first 5 are listed here.
|
09210d3 to
9bb650f
Compare
chihsuan
added
[Status] Needs Review
layoutd
left a comment
layoutd
left a comment
There was a problem hiding this comment.
LGTM. Followed the testing instructions: full premium-analytics suite, typecheck and lint-changed all green on the branch. Also went through each rewritten call site against the original: toValidDay was identical to the shared toDay, and the null vs undefined swap in readRowDate is absorbed by its one caller (if ( ! date )). Clicked through Popular days and Post views on a local site too and nothing moved.
| Significance: patch | ||
| Type: changed | ||
|
|
||
| Report dates: read every round-trip-validated date label and day bound through one helper. |
There was a problem hiding this comment.
This reads as implementation detail. Since the PR says no behavior changes, the changelog guideline in AGENTS.md points at an empty entry with a -c comment. Not blocking
2 participants
Fixes WOOA7S-2079 (items 3, 4, 7 and part of 5; item 1 is #52064)
Proposed changes
parseExactLabel( label, labelFormat )intopackages/datetimeand route the five copies of theparse+isValid+ round-tripformatidiom through it, along with the two copies of itsnew Date( 2001, 0, 1 )reference date.toValidDayinuse-post-views.ts, which duplicated the sharedtoDaycharacter for character. Its sibling widget already imported the shared one.'00:00:00'/'23:59:59'day bounds asDAY_START_TIME/DAY_END_TIMEinstats/utils.ts, next to the three files that use them.No behavior changes, so nothing in
readme.txtand no plugin changelog entries. The only test file edited gains assertions; none was moved or weakened.Related product discussion/links
Does this pull request change what data or activity we track or use?
No.
Testing instructions
Nothing to click. This is an internal refactor with no user-visible surface, so the check is that the existing suites still pass without any assertion being edited.
jp test js packages/premium-analytics(or, scoped:PA_TEST_GROUPS=1 TZ=UTC npx jest --config=tests/jest.config.cjs packages/datetime packages/data widgets/post-views widgets/popular-days packages/widgets-toolkit), 166 suites, 1531 tests.pnpm run typecheckandpnpm run lint-changed.Date | nulltoundefinedconversion inreadRowDateand the reference-date consolidation.